Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KafkaSinkCluster: authorize_scram_over_mtls #1605

Merged
merged 1 commit into from
May 14, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented May 2, 2024

closes #1600

Possibly useful resources:

This PR introduces a new optional config to KafkaSinkCluster:

            authorize_scram_over_mtls:
              mtls_port_contact_points: ["172.16.1.2:9094"]
              tls:
                certificate_authority_path: "tests/test-configs/kafka/tls/certs/localhost_CA.crt"
                certificate_path: "tests/test-configs/kafka/tls/certs/localhost.crt"
                private_key_path: "tests/test-configs/kafka/tls/certs/localhost.key"
                verify_hostname: true

The documentation added in docs/src/transforms.md explains what this feature does and why we want it, so please refer to it.

In terms of implementation decisions:

I looked into two crates to provide the SCRAM implementation

  • rsasl crate
    • CI is broken
    • supports SCRAM, but no obvious way to use just SCRAM
  • sasl crate
    • very easy to use just the SCRAM parts we need.
    • Maintained within the XMPP repo
      • negative: sasl crate may be tailored more towards the needs of XMPP
      • positive: the XMPP maintainers are available to maintain the sasl crate.

In conclusion the sasl crate looked like a clear winner, so I went with it.

I've left the hardcoded 4 second delay in, it will be addressed along with caching delegation tokens in a follow up PR: #1618

@rukai rukai force-pushed the scram_over_mtls branch 13 times, most recently from 03552b0 to aae94bf Compare May 9, 2024 04:22
@rukai rukai force-pushed the scram_over_mtls branch 5 times, most recently from de870d3 to ad7f208 Compare May 10, 2024 00:05
@rukai rukai marked this pull request as ready for review May 10, 2024 00:25
@rukai rukai requested a review from conorbros May 10, 2024 00:36
@conorbros conorbros merged commit 59eda05 into shotover:main May 14, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shotover supports topology where: client <-scram-> shotover <-mTLS->+<-scram-> kafka
3 participants